tools/backup-projects.sh (1)

353-357: umask 077 is process-global — it silently affects all subsequent file/directory creation.

umask changes apply to the calling process and are inherited by all child processes. Setting it here means every file/directory created after this point (log files in sync_git_repos, the LOG_DIR at line 357, zip temp files, etc.) will silently receive restrictive 600/700 permissions. This is benign for a personal backup script but obscures intent.

The LOG_DIR at line 357 will also be created as 700 with no explicit chmod, which may be surprising.

Consider saving and restoring the umask to scope the tightened mask to just the directory creation step:

♻️ Proposed scoping via save/restore

 if [[ "$DRY_RUN" != true ]]; then
     # Ensure secure permissions for backup directory (700)
+    _old_umask=$(umask)
     umask 077
     mkdir -p "$BACKUP_TEMP_DIR"
     chmod 700 "$BACKUP_TEMP_DIR"
+    umask "$_old_umask"
     mkdir -p "$LOG_DIR"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/backup-projects.sh` around lines 353 - 357, The script sets umask 077
globally which unintentionally tightens permissions for all subsequent creations
(affecting LOG_DIR, zip temp files, and functions like sync_git_repos); to fix,
scope the restrictive mask around just the backup dir creation by saving the
current umask, setting umask 077, creating/chmod-ing BACKUP_TEMP_DIR, then
restoring the saved umask before continuing so LOG_DIR and other files are
created with their intended permissions; reference the umask call and the
mkdir/chmod operations for BACKUP_TEMP_DIR and the subsequent mkdir for LOG_DIR
when applying this change.

tests/verify_backup_permissions.sh (1)

1-36: Several test robustness gaps.

Three issues worth addressing together:

1. SCRIPT_PATH assumes pwd is the repo root (Line 9). Running the test from any other directory silently targets the wrong path or fails with no explanation. Use the script's own location instead:

   -SCRIPT_PATH="$(pwd)/tools/backup-projects.sh"
   +SCRIPT_PATH="$(cd "$(dirname "$0")/.." && pwd)/tools/backup-projects.sh"

2. stderr from the backup script leaks to the terminal (Line 36). Warning messages (yq not installed, git output, etc.) pollute test output. Use >/dev/null 2>&1.

3. set -e + no yq = silent, opaque failure (Lines 2 and 36). If yq is not installed the backup script falls back to default folders, archives nothing, exits 1, and the test exits immediately via set -e with no diagnostic. Add a prerequisite guard:

♻️ Proposed fixes

 #!/bin/bash
-set -e
+set -euo pipefail

 # Setup test paths
 TEST_DIR="$(mktemp -d)"
 TEST_SOURCE="$TEST_DIR/source"
 TEST_BACKUP_DIR="$TEST_DIR/backups"
 TEST_CONFIG="$TEST_DIR/config.yaml"
-SCRIPT_PATH="$(pwd)/tools/backup-projects.sh"
+SCRIPT_PATH="$(cd "$(dirname "$0")/.." && pwd)/tools/backup-projects.sh"

 # Cleanup on exit
 trap 'rm -rf "$TEST_DIR"' EXIT

+# Prerequisites
+if ! command -v yq &>/dev/null; then
+    echo "SKIP: yq is required but not installed"
+    exit 0
+fi
+
 ...

 # Run backup script
-"$SCRIPT_PATH" backup --config "$TEST_CONFIG" >/dev/null
+"$SCRIPT_PATH" backup --config "$TEST_CONFIG" >/dev/null 2>&1 || {
+    echo "FAIL: backup script exited non-zero"
+    exit 1
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/verify_backup_permissions.sh` around lines 1 - 36, The test assumes
SCRIPT_PATH via pwd, leaks stderr, and can fail silently under set -e if yq is
missing; fix by resolving the script path relative to the test file (use the
test script's directory / dirname reference to locate tools/backup-projects.sh
rather than pwd), add a prerequisite guard that checks for required binaries
(check for yq and any other required commands and print a clear error + exit if
missing), and suppress the backup script's stdout/stderr in the run invocation
by redirecting both stdout and stderr (use >/dev/null 2>&1) when calling
"$SCRIPT_PATH" backup --config "$TEST_CONFIG". Ensure these changes reference
SCRIPT_PATH, TEST_CONFIG, and the initial set -e behavior so the guard runs
before the script invocation.